quic: add support for HTTP/3 datagrams#64234
Open
pimterry wants to merge 1 commit into
Open
Conversation
Signed-off-by: Tim Perry <pimterry@gmail.com>
Collaborator
|
Review requested:
|
nodejs-github-bot
added
c++
| // A pending stream has no id yet, so there is nothing to bind to. | ||
| if (stream->is_pending()) { | ||
| return args.GetReturnValue().Set(BigInt::New(env->isolate(), 0)); | ||
| } |
Contributor
There was a problem hiding this comment.
Well for webtransport, it would be the right behavior, to store it in a list (with maximum capaticity). This of course could be done also outside.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With nghttp3 released and updated here, we can now cleanly add proper HTTP/3 datagram support to fix #63891. This PR:
stream.sendDatagramandstream.ondatagramsupport on HTTP/3 streams.The resulting API is a little awkward (both H3 & QUIC methods present everywhere, but throwing in different cases) due to the mixed QUIC/H3 API as already discussed, but I think it's worthwhile opening in the meantime anyway so people can start playing with this, and so we can review it standalone.
The API will conflict with the HTTP/3 split PR that restructures related areas, but the internals should be the same in any case (just moved around) so it'll be easy to slide into whatever final structure we end up with there.
This doesn't support HTTP/3 datagrams for 0RTT on the client side. The RFC would require us to persist the HTTP/3 SETTINGS client-side to validate datagrams are supported to do so, but we currently don't persist HTTP/3 SETTINGS at all. Right now for all HTTP/3 0RTT we're just falling back to the HTTP/3 defaults instead (as allowed by the spec) but the datagram default is off. We could explore that later if we want, but completely rejigging HTTP/3 session resumption to enable this seemed more effort than it's worth for now. Server acceptance of inbound 0RTT datagrams from non-Node clients should work fine but can't be covered in the tests due to this.